================================================================================
Bug: #5173
================================================================================
Bug #5173  	MyODBC: extend_buffer function returns wrong value when to == NULL
Submitted:	23 Aug 1:42pm 	Modified:	20 Dec 2:58am
From:	Mark Smith 	User type:	User
Status:	Verified
Priority:	High 	Severity:	Serious
		Showstopper:	yes
Category:	Connector/ODBC
Version:	3.51.07 	OS:	x86 Linux (Fedora Core 2)
Changeset:	
Internal info:	
Not in released versions:	no 	Affects paying customer:	no
Associated support issues:	
View Add Comment Files Developer Edit Submission View Progress Log

[23 Aug 1:42pm] Mark Smith

Description:
The extend_buffer() function in driver/execute.c returns the wrong value when
the to parameter is passed in as NULL.  In fact, it returns a pointer to memory
which is outside net->buff.  The problem is that the local variable nead is
initialized to zero but never set to the number of bytes required in the to ==
NULL case.

This can lead to problems when, for example, the client has lost its connection
to the MySQL server (apparently someone resets the statement buffer to NULL when
that happens).  This leads to crashes when using bound parameters.

How to repeat:
Create a test program that does something like this:
  1. Open a connection to a server using SQLDriverConnect(), etc.
  2. Use SQLBindParameter() to bind one simple string parameter.
  3. Do a simple SELECT by calling SQLExecDirect().
  4. Reset the statement handle by calling SQLCloseCursor(hstmt) and
SQLFreeStmt(hstmt, SQL_RESET_PARAMS).
  5. Pause (e.g., do a gets() to wait for a Return).
  6. Repeat steps 2-4 two more times.

Then stop the MySQL Server during the Pause.  You will likely see a SIGSEGV.  If
run under Valgrind you will see a series of "Invalid write" errors, e.g.,

==13185==
==13185== Invalid write of size 1
==13185==    at 0x1B9046DA: memcpy (mac_replace_strmem.c:283)
==13185==    by 0x1BAAA6CE: add_to_buffer (execute.c:169)
==13185==    by 0x1BAAA897: insert_params (execute.c:231)
==13185==    by 0x1BAAB58C: my_SQLExecute (execute.c:595)
==13185==    by 0x1BAAB1EB: SQLExecute (execute.c:519)
==13185==    by 0x1B919D16: ??? (execute.c:194)
==13185==    by 0x1B919F9D: SQLExecute (execute.c:279)
==13185==    by 0x804917E: ODBC_Test (iodbctest.c:419)
==13185==    by 0x80496D8: main (iodbctest.c:631)
==13185==  Address 0x1B9B8477 is 1 bytes before a block of size 8192 alloc'd
==13185==    at 0x1B904A7C: malloc (vg_replace_malloc.c:131)
==13185==    by 0x1B90546B: realloc (vg_replace_malloc.c:189)
==13185==    by 0x1BABB1BC: my_realloc (in
/usr/local/packages/myodbc-3.51-from-src/lib/libmyodbc3_r-3.51.07)
==13185==    by 0x1BAAA5AA: extend_buffer (execute.c:146)
==13185==    by 0x1BAAA69B: add_to_buffer (execute.c:167)
==13185==    by 0x1BAAA897: insert_params (execute.c:231)
==13185==    by 0x1BAAB58C: my_SQLExecute (execute.c:595)
==13185==    by 0x1BAAB1EB: SQLExecute (execute.c:519)
==13185==    by 0x1B919D16: ??? (execute.c:194)
==13185==    by 0x1B919F9D: SQLExecute (execute.c:279)
==13185==    by 0x804917E: ODBC_Test (iodbctest.c:419)
==13185==    by 0x80496D8: main (iodbctest.c:631)

Suggested fix:
Here is one possible fix:

--- execute.c.orig      2004-03-16 17:45:31.000000000 -0500
+++ execute.c   2004-08-23 16:15:01.801915528 -0400
@@ -126,18 +126,18 @@
 {
   ulong nead= 0;
   DBUG_ENTER("extend_buffer");
   DBUG_PRINT("enter",("current_length: %ld  length: %ld  buffer_length: %ld",
                      (ulong) (to - (char*) net->buff),
                      (ulong) length,
                      (ulong) net->max_packet));
 
-  if (!to ||
-      (nead= (ulong) (to - (char*) net->buff)+length) > net->max_packet-10)
+  nead = (ulong) (to - (char*) net->buff)+length;
+  if (!to || nead > net->max_packet-10)
   {
     ulong pkt_length= (nead+8192) & ~(8192-1);
     uchar *buff;
 
     if (pkt_length > max_allowed_packet)
     {
       DBUG_PRINT("error",("Needed %ld but max_allowed_packet is %ld",
                          pkt_length,max_allowed_packet));

[20 Dec 2:58am] Hartmut Holzgraefe

the proposed patch seems valid to me as 
assignments within a multicondition if() depend to much on lazy evaluation
order
to be considered best practice anyway (or even assignments within if() in
general)

================================================================================

Create a test program that does something like this:
  1. Open a connection to a server using SQLDriverConnect(), etc.
  2. Use SQLBindParameter() to bind one simple string parameter.
  3. Do a simple SELECT by calling SQLExecDirect().
  4. Reset the statement handle by calling SQLCloseCursor(hstmt) and
SQLFreeStmt(hstmt, SQL_RESET_PARAMS).
  5. Pause (e.g., do a gets() to wait for a Return).
  6. Repeat steps 2-4 two more times.


create table test1 (ch1 char(40))
drop table test1
select ch1 from test1
select ch1=? from test1


SQLBindParameter:
In:				StatementHandle = 0x003A1910, ParameterNumber = 1, 
				InputOutputtype = SQL_PARAM_INPUT=1, ValueType = SQL_C_CHAR=1, ParameterType = SQL_CHAR=1, ColumnSize = 0, 
				DecimalDigits = 0, ParameterValuePtr = SQL_NULL_HANDLE, BufferLength = 0, 
				StrLen_or_IndPtr = SQL_NTS=-3, SQL_LEN_DATA_AT_EXEC = FALSE, Buffer Size = 300
				Return:	SQL_SUCCESS=0

SQLExecDirect:
In:				Statementhandle = 0x003A1910, 
				StatementText = "select ch1 from test1", Statementlength = 21
				Return:	SQL_SUCCESS=0

================================================================================

Testing on Windows no Problems, without stops server!

Whats a server version the customer are using?

build patch with suggeded fixed fom customer!

The ODBC functions are calls  was different between odbc 2.0 and 3.0

Servers: 4.0.18 works correct
	 4.0.23 works correct
================================================================================
